-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Convert LayoutGridField to function components #4807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides moving everything around so that things aren't used before they are defined, I'd say it looks good. IF you do decide to go with a hook (in @rjsf/utils), it probably makes sense to also update my ObjectField refactor to use it too.
| "homepage": "https://github.com/rjsf-team/react-jsonschema-form", | ||
| "publishConfig": { | ||
| "access": "public" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YAY! This helps a lot
…form into layout-grid-fn
LayoutGridField: - Deep compare memoize the fieldPathId - Move helper functions in LayoutGridField - Update docs comments in LayoutGridField - Attach TEST_IDS to LayoutGridField function Additionally: - Update migration guide per rjsf-team#4808
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm approving but there are a few simple changes that you can make first.
| } = getSchemaDetailsForField<T, S, F>(registry, name, initialSchema, formData, fieldPathId); | ||
| const memoFieldPathId = useDeepCompareMemo<FieldPathId>(fieldIdSchema); | ||
|
|
||
| if (uiComponentProps.rendered) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It kind of sucks that the rules of hooks forces us to call that rather expensive getSchemaDetailsForField() first
- Move LayoutGridFieldChildren component definition above usage - LayoutGridFieldComponent": Simplify props destructure - Fix issue with the nx dependency graph - dependencies were not tracked
Reasons for making this change
Convert the LayoutGridField into function components
Checklist
npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:updateto update snapshots, if needed.